Delete and expire library-owned experience data (story 4.5) - #17
Merged
Merged
Conversation
Revocation stops reads; this adds the erasure. One authorized, atomic, scope-safe operation removes every payload-bearing trace of an experience across seven tables and leaves a payload-free tombstone that prevents the ID being written again. A retention sweep runs the same operation in bounded batches; default retention is indefinite and scheduling belongs to the host. Erasure is an authorized exception to the append-only guards, not a suspension of them. A new 0010 replaces the trigger functions so they recognise one transaction-scoped marker set only inside the purge function; 0006 is journaled and untouched, every ENABLE ALWAYS binding survives, and the guard stays armed for every other statement in every other session throughout. This replaces the documented runbook that disabled a trigger table-wide. It is an auditability mechanism, not a privilege boundary, and the documentation says so: a custom GUC is settable by any session and the guards already do not bind a role that can ALTER TABLE. What it buys is one code path, inside one transaction, with the guard never switched off and never left off across a failure. The purge functions are SECURITY DEFINER, so EXECUTE is revoked from PUBLIC and granted explicitly. Without that revoke a SELECT-only role could erase any record in any tenant. Writers that gate on deleted_at now take a key-share lock on the record row, so an in-flight embedding, grant or feedback write serializes against a purge instead of against a snapshot taken before it committed. experience_records gains DELETE and TRUNCATE guards: the purge updates that row into a tombstone and never deletes it, so nothing legitimate removes it, and a removal would orphan the audit trail and free the ID for reuse under another scope's grants. What erasure does not reach is stated rather than implied: backups, replicas, WAL, exported telemetry, external artifacts, dead index entries until VACUUM, and the previous row version in the heap, which still carries the erased text until the table is vacuumed. Also closes the cross-scope squat by mixing Scope into the derived experience ID -- a breaking change to record identity, taken now because it is free before release -- and injects TimeProvider into the stores that lacked one. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Revocation stops reads. This adds the erasure: one authorized, atomic, scope-safe operation that removes every payload-bearing trace of an experience across seven tables and leaves a payload-free tombstone preventing the ID from being written again — plus a bounded retention sweep running the same operation.
Erasure is an exception to the append-only guards, not a suspension of them
0010replaces the trigger functions so they recognise one transaction-scoped marker set only inside the purge function.0006is journaled and untouched, everyENABLE ALWAYSbinding survives, and the guard stays armed for every other statement in every other session throughout. This replaces the previously documented runbook, which disabled a trigger table-wide — leaving the audit guard off for every connection in the pool, and off permanently if the transaction died in between.It is an auditability mechanism, not a privilege boundary, and the docs say so in those words. A custom GUC is settable by any session, and the guards already do not bind a role that can
ALTER TABLE— which is the application role. What it buys is one code path, one transaction, and a guard that is never switched off.What three reviewers found
One reviewer stood up a real database and attacked the purge with SQL rather than reasoning about it. That found what code reading did not:
SECURITY DEFINERpurge functions had noREVOKE EXECUTE … FROM PUBLIC, so aSELECT-only reporting role — deniedDELETEandUPDATEon every table — could permanently erase a record in another tenant. Fixed, with a test using a real non-owner role.Deleted. An embedding whose vector is a searchable derivative of the erased text; a live 90-day grant over a spent ID; a reviewer identity and free-text rationale about the erased record, in an append-only table. Every writer that gates ondeleted_atnow takes a key-share lock and serializes against the purge.DELETE FROM experience_recordswas completely unguarded — orphaning the audit trail and freeing the ID, so a reinsert inherited the old grants. Now refused outright: the purge updates that row into a tombstone and never deletes it.VACUUM— and the one sentence about residue reassured the opposite. Corrected; it is the sentence that matters most to anyone with a real erasure obligation.The mutation reviewer explained why the worst defects had to be found by hand: there was no concurrency test anywhere in this story, though the suite has the pattern three times elsewhere. There are now six.
Also in this change
Closes the cross-scope squat by mixing
Scopeinto the derived experience ID — a breaking change to record identity, taken now because it is free before release. InjectsTimeProviderinto the stores that lacked one. The destructive grant purge usesLEAST(host clock, clock_timestamp()), so a skewed host clock cannot destroy a grant the database still considers live.Documentation states what erasure does not reach, that a sweep matches one scope exactly (the one operation whose failure mode is a missed retention obligation reported as success), which tombstone protections are schema-enforced and which are adapter-enforced, and a
CREATE INDEX CONCURRENTLYrunbook for the three new indexes, which build under a lock on the largest table this library writes.Tests: 1295 → 1333, zero warnings under
TreatWarningsAsErrors.🤖 Generated with Claude Code